fix: design-system audit — motion, interaction states, focus & accessibility#856
fix: design-system audit — motion, interaction states, focus & accessibility#856ravisuhag wants to merge 61 commits into
Conversation
Compose the shared easing, duration, pressed-scale and focus-ring tokens so components stop hand-rolling their own timings and curves.
Step body letter-spacing and h1-h4 line-heights so they change monotonically across sizes and follow Inter's own tracking curve.
Give numeric columns tabular figures so digits align, alongside the shared row-selection and sticky-header styling that lives in the same two stylesheets.
A slightly faster spinner raises perceived load speed. Recomputed all 8 pole animation-delays to keep the same 1/8-cycle stagger ratio at the new duration, so the sweep still rotates evenly.
Centering used translateX(-50%), which would collide with any transform-based entrance. Move centering to inset+margin-inline:auto (safe since the root is width:fit-content) so transform is free for motion. Add a fade-in for everyone, plus an 8px rise from the anchored edge under prefers-reduced-motion: no-preference. Exit stays instant (no mount controller exists to hold the node past removal).
The error handler swapped src to the fallback with no re-entry guard -- if the fallback URL itself 404s, onError fires again and re-assigns the same failing src, looping requests indefinitely. Track a one-shot ref so the fallback is only ever assigned once; a second failure falls through to showing the alt/broken rendering instead. Both Image and Avatar's photo popped in abruptly with no load transition. Image now tracks load state via a useLayoutEffect that checks node.complete on mount/src-change -- already-decoded (cached or SSR-painted) images stay visible with no fade; genuinely loading images fade in over 150ms on load. Avatar's image only mounts once Base UI's probe reports loaded, so its fade is a one-shot keyframe animation, skipped entirely for cache-fast loads (<100ms between loading and loaded) so a quick load isn't artificially delayed. Reduced motion shows both instantly at full opacity, never invisible.
A display:block heading already fills its container in normal flow; the explicit width:100% only mattered in flex/grid contexts, where it made a heading claim the full row/track and push siblings around. Ellipsis truncation is the only variant that needs an explicit width, and it already declares its own -- removing the base rule loses nothing there. BREAKING (mild): consumers relying on the base width:100% inside flex/ grid parents (e.g. a centered heading, or a heading used as a full-row flex child) will see it shrink to content width. One-line fix on their side: set width:100% via className where the old behavior was intentional.
The action button was the bar's only interactive element but had no hover, press, or focus feedback. Add opacity-step hover/press states and a currentColor focus ring that stays visible against the bar's coloured backgrounds.
…hiding it Both variants hid the shared Tabs.Indicator and painted the active state directly on the trigger, so switching tabs snapped instantly instead of gliding like the default segmented variant. The indicator already tracks the active tab via Base UI's --active-tab-* variables for free -- hiding it threw that away. Restyle the indicator per variant instead: standalone becomes the active chip fill (background + border) gliding between tabs, with the active trigger going transparent so the pill shows through (the same trick the segmented variant already uses); plain becomes a 1px underline sliding along the bottom edge, positioned via the trigger's own --active-tab-height so it lines up with the reserved border-bottom exactly. Both inherit the indicator's existing --rs-duration-moderate/--rs-ease-in-out transition, gated behind reduced motion, with no new media query needed.
Breadcrumb links changed colour instantly on hover while the rest of the library eases. Add a short colour transition, gated behind prefers-reduced-motion, so the hover matches every other interactive element.
Popover -- a larger floating surface than the tooltip -- cast a lighter shadow (--rs-shadow-soft) than the tiny tooltip (--rs-shadow-lifted), so elevation read backwards when both were on screen. Match menu, a peer surface of similar size, which already uses --rs-shadow-lifted.
overflow: scroll forced a permanently visible scrollbar gutter on platforms with classic scrollbars, even when content didn't overflow. Use auto. Also document via JSDoc that SidePanel intentionally has no enter/exit animation -- it's persistent layout chrome like Sidebar, not an overlay.
The scrollbar grew its whole track on hover, reflowing the page each time, and the thumb never showed it was grabbed. Keep the track a constant, invisible width so layout never shifts; draw the thumb thinner at rest and grow it to full width on hover; and deepen its colour one step on hover and again while dragging.
The positioner transitioned top/left/right/bottom and the popup transitioned width/height on every open -- but Base UI only writes inset positioning and drives --popup-width/--popup-height when a PreviewCard.Viewport child triggers its adaptiveOrigin mode. In the common case (no Viewport), Base UI positions purely via transform, so the four inset transitions were dead weight forcing unnecessary transition-list overhead on every reposition. Split into two rulesets: the default path transitions transform + opacity only (compositor-only), while `:has(.viewport)` keeps the inset/size transitions where they're the actual morph mechanism.
A sticky header drew its bottom hairline even at rest, before anything had scrolled beneath it. Drive the hairline from a scroll-linked animation so it fades in over the first bit of scroll, gated behind a real scroll container so the plain table keeps its static separator.
Resting day cells used radius-2 while hover/selected/range-cap states all used radius-5, so sweeping the pointer across the grid made each cell's shape pop between rounded-square and pill. The radius-2 resting state was never actually visible (the cell's background matches the calendar root's), so unify on radius-5 -- zero visible states change, one invisible one gets fixed. Hover/selection highlights stay instant, per the audit's explicit rejection of transitions on direct-manipulation surfaces. Loading hard-swapped the whole table for a differently-sized Skeleton in one frame, flashing on fast loads and jumping the popover height. Keep the table mounted and overlay the skeleton, cross-fading via opacity/visibility (~150ms, instant under reduced motion) so the grid never disappears and the height never moves.
Dimming a link to 80% opacity on hover is a weak signal and makes the text harder to read at the exact moment the user is targeting it. The underline geometry now always exists (text-decoration-color: transparent at rest) so hover is a pure color fade to currentColor -- zero layout shift, works for every Text color variant automatically. Press gets a brief variant-agnostic dim (:active, opacity 0.7) instead of a hardcoded color, since a fixed active color would clobber non-accent variants. Timing moves from a hardcoded 0.2s to --rs-duration-fast.
Proportional digits meant amounts jittered as values changed and misaligned in table columns. Add a CSS module with font-variant-numeric: tabular-nums as the default; className/style still merge through as the opt-out escape hatch.
The floating copy button had no transition of its own duration (it inherited the interactive tier's opacity fade), so its reveal moved at hover-feedback speed instead of a dedicated reveal timing. Re-compose the IconButton base transition and override just the opacity entry with --rs-duration-fast/--rs-ease-out. The :focus-visible reveal rule (added by the earlier focus-indicators pass) is merged into the same selector as the hover reveal for clarity.
Row selection was wired up in data-table and data-view-beta (both set
data-state="selected") but no CSS anywhere styled it -- selecting a
row changed nothing on screen. Add an accent-subtle background for
selected rows (wins over hover/pressed) and a one-step-darker pressed
background for clickable rows, instant (no transition, matching the
house rule for direct-manipulation highlights like menu items).
Virtualized cells painted an opaque background that covered the row's
color entirely -- add `.virtualRow .virtualCell { background: inherit }`
so selection/hover/press show through in virtualized mode, same as the
non-virtualized cells already do.
data-view (no selection support) gets the same pressed-row treatment.
The bare Table primitive had no row-state hooks at all. Table.Row now
accepts an `interactive` prop (cursor + hover + pressed) and responds
to a consumer-set data-state="selected", with cells inheriting the row
background through the opaque .cell rule.
…bled cursor Hovering an off toggle looked identical to the on state -- the control lied about whether it was selected. Give hover a lighter tint and pressed (Base UI's toggled-on state) a deeper, inset-shadowed look in both standalone and group mode; hovering an already-on toggle now deepens it one more step instead of reverting to the hover tint. The content background and inset shadow had no transition at all (only color did), so state changes snapped. Add a fast-tier transition for both. pointer-events: none on disabled toggles suppressed the declared cursor: not-allowed, so disabled toggles gave zero affordance. Restore pointer-events so the cursor shows, with explicit hover neutralizers so a disabled-but-off toggle doesn't pick up hover styling (clicks stay blocked natively via the disabled <button>).
The select positioner painted an opaque square that flashed behind the popup as it scaled, and multi-select options nested a real focusable checkbox inside each row, adding a stray tab stop. Drop the positioner background and use a presentational check indicator; the trigger shows a static up/down caret to signal it opens a menu.
A chip with onClick rendered as a <span onClick>: no keyboard activation, not in the tab order, no focus ring, and cursor: default even though it was clickable. Render a <button type="button"> when onClick is passed and the chip isn't dismissible (a button containing the dismiss button would be invalid nested-interactive HTML, so dismissible chips keep the span). Interactive chips get cursor: pointer, inherit the app font, and a focus-visible accent ring.
The Field error popped in with no transition and replaced the description in one jarring jump, and the DatePicker set an error that was never shown. Cross-fade the error in over the description so the swap is smooth, and render the DatePicker error as reserved-space helper text below the input.
…es in The 2D area thumb was positioned via left/top on every pointermove, forcing layout on each frame of a drag. Drive it from two CSS custom properties (--thumb-x/--thumb-y, unitless 0..1 fractions set from JS) composed with container query units and the existing -50% self-centering transform -- compositor-only, no transition on transform so the drag stays 1:1 with the pointer. Also add touch-action: none / user-select: none to .selectionRoot (the sliders already had this; the 2D area didn't, so dragging on touch also scrolled the page), and a fast opacity transition on the thumb so its first-render reveal fades instead of popping.
.nav-text declared an opacity transition, but the collapsed state set display: none, which can't transition and short-circuited it -- labels popped out of existence while the frame animated its width. Same for any consumer content marked data-collapse-hidden. Replace display:none with opacity + visibility (visibility keeps collapsed content out of the accessibility tree and unfocusable, same intent as display:none, but participates in the transition). Also fix the resize handle's cursor: it only click-toggles collapse, but showed ew-resize, promising a drag that doesn't exist. Use pointer on both left- and right-positioned sidebars (per the audit's explicit decision not to implement drag-to-resize).
Uncontrolled dismiss returned null on the next render, so the callout vanished in one frame and content below it snapped up by its full height. There was no enter transition either. Wrap the callout in a transition shell driven by a data-state attribute (open/closing/closed). Dismiss now plays a 200ms fade + grid-row height collapse before unmounting, so content below glides up instead of snapping. Mount fades in over the same duration. Reduced motion is instant both ways. The controlled API (onDismiss provided) is unchanged -- the consumer still owns removal.
Tooltip animated with one-shot @Keyframes: not interruptible (restarts from zero instead of retargeting), no exit at all (popup unmounted instantly), a dead unused fade keyframe, and a dead transform-origin (every keyframe was a pure translate, origin-independent). Replace with [data-starting-style]/[data-ending-style] transitions at --rs-duration-fast/--rs-ease-out, keeping the exact per-side and per-align 2px directional offsets the keyframes used. Enter and exit now mirror each other, rapid hover toggling retargets smoothly instead of restarting, data-instant still skips the animation via transition-duration: 0, and reduced motion gets an opacity-only fade (previously nothing, since every rule lived inside the no-preference media query).
Four independent gaps: - Toast starting-style set a transform but no opacity: 0, so under reduced motion (where the transform never transitions) toasts popped in at full opacity instead of fading. - Navbar's hide-on-scroll transform had no reduced-motion fallback at all, so the bar teleported in/out; add an additive `reduce` block that fades it instead. - Spinner poles had no static opacity, so reduced motion (where the spin animation never applies) rendered a static asterisk instead of a graded ring that still reads as loading. - Dialog and command's backdrop blur had no prefers-reduced-transparency guard; add one with a heavier solid scrim (alert-dialog shares dialog's stylesheet, so it's covered too).
…uced motion The popup kept its 450ms transform transition active while swiping, so it eased toward the finger instead of tracking it directly (the backdrop already zeroed its own transition). Add transition: none on [data-swiping]. The five swipe-strength exit calcs hardcoded 400ms while enter runs 450ms via --rs-duration-drawer -- non-swipe exits ran measurably faster than opens for no reason. Use the token so enter/exit match and retuning the drawer duration doesn't require finding five literals. The panel had no transition outside the motion media query, so under prefers-reduced-motion it popped open/closed instantly. Add a base opacity fade (with matching [data-starting-style]/[data-ending-style] opacity: 0) so reduced motion gets a fade instead of a teleport, and move the per-side slide transforms into the no-preference block alongside the transition they animate.
Two sides of the same house rule: direct-manipulation highlights are instant, single controls ease. .nav-item eased its hover/press background through --rs-transition-interactive. The ease came from main (tokenized in 1915309) and made the sidebar the only pointer-following list whose highlight trailed the cursor -- menu items, select options, command items, and table rows are all instant. Drop the transition. The More trigger composes .nav-item, so it now matches the popup items it opens, which were already instant. The accordion trigger is the opposite case: a single full-width control whose hover/press background flipped instantly while the same component eases its chevron and panel. Give it the shared interactive token inside the file's existing reduced-motion block, matching the button pattern (plan 12, accordion section). Plan 12's other targets were dropped: calendar day cells are a sweep surface (instant, per the rule above), and the sidebar More item and group header only needed to match .nav-item -- which is now instant.
Select.Item hand-rolled its multi-select indicator: a styled span plus
a local copy of the checkmark svg, conditionally mounted -- so the fill
snapped between white and accent and the checkmark popped in and out
with no fade. Combobox, the sibling popup surface, renders the real
Checkbox inside its items and gets the transitions, keep-mounted
fade-out, and reduced-motion handling for free.
Mirror combobox exactly: a Checkbox with checked={state.selected} and
no onCheckedChange (fully controlled and inert; the click bubbles to
the item, which owns selection), delete the local CheckMarkIcon (its
path was byte-identical to Checkbox's), and drop the three
.checkIndicator rules. Checkbox's default large size is the same
--rs-space-5 box with the same tokens, so rows do not shift.
This deliberately reverses 684eade, which had replaced the nested
Checkbox with the presentational span to avoid a stray tab stop. In
practice the checkbox's tabIndex={0} span is not reachable -- the popup
manages focus on the options and Tab closes it -- and combobox has
shipped the nested Checkbox all along. Consistency between the two
surfaces wins; tightening the nested-checkbox semantics in both at
once stays open as a follow-up (plan 15).
Tests: multi-select options contain exactly one checkbox whose
aria-checked follows selection, and a click fires onValueChange
exactly once.
The floating copy button's icon box was 12x16 (--rs-space-4 wide, --rs-space-5 tall). Radix icons use a square viewBox, so the glyph rendered ~1.33x taller than wide on every floating copy button. Use --rs-space-5 for both sides. Keeping the 16px height means the pill's height and layout do not change, and 16px is the natural glyph size of the size-2 IconButton that CopyButton renders, so the floating variant stops fighting the component's own sizing (plan 16).
dcaaa30 gave the DatePicker a reserved-space error line below its input (invisible until an error shows) so appearing errors never shift the layout under the field. Standalone that is right, but FilterChip embeds the DatePicker inline in a 24px chip row: the invisible line inflated the date-field wrapper to 38px, and flex centering pushed the input ~7px above the chip's centerline -- "Select date" poked out over the chip border next to a correctly centered label and operator. Hide the error line inside the chip's date wrapper, same as the chip already strips Input helper text there. Date errors inside chips keep their existing signal: the danger border on the input wrapper.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/raystack/components/headline/headline.module.css (1)
31-41: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider renaming physical class names to logical ones.
Since the alignments are now using logical properties (
startandend) for better RTL support, you might want to rename the utility classes from.headline-align-leftand.headline-align-rightto.headline-align-startand.headline-align-end. This avoids confusion where a-leftclass visually aligns text to the right in an RTL context.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/raystack/components/headline/headline.module.css` around lines 31 - 41, Rename the alignment utility classes from .headline-align-left and .headline-align-right to .headline-align-start and .headline-align-end, while preserving their existing text-align values. Update all references to these classes so the public utility names consistently describe the logical alignment behavior.packages/raystack/styles/typography.css (1)
4-4: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse string notation for the
@importURL.The static analysis tool Stylelint flags the use of
url("...")for this import. Prefer using the direct string notation to resolve theimport-notationlinter error.♻️ Proposed fix
-@import url("https://fonts.googleapis.com/css2?family=JetBrains+Mono:ital,wght@0,400;0,500;1,400;1,500&display=swap"); +@import "https://fonts.googleapis.com/css2?family=JetBrains+Mono:ital,wght@0,400;0,500;1,400;1,500&display=swap";🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/raystack/styles/typography.css` at line 4, Update the font `@import` declaration in typography.css to use direct quoted-string notation for the Google Fonts URL instead of the url() wrapper, preserving the existing URL and import behavior.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/raystack/components/announcement-bar/announcement-bar.module.css`:
- Around line 62-64: Update the .action-btn:active pressed-state styling so its
scale transform is disabled when prefers-reduced-motion is reduce, while
preserving the existing transform for users with no motion preference. Apply the
same reduced-motion handling to the related styles referenced at lines 71–78.
In `@packages/raystack/components/data-view/components/list.tsx`:
- Around line 357-363: Update handleRowKeyDown to ignore bubbled keydown events
unless event.target is the row element itself, before handling Enter or Space.
Preserve the existing preventDefault and handleRowActivate behavior for direct
row keyboard activation.
---
Nitpick comments:
In `@packages/raystack/components/headline/headline.module.css`:
- Around line 31-41: Rename the alignment utility classes from
.headline-align-left and .headline-align-right to .headline-align-start and
.headline-align-end, while preserving their existing text-align values. Update
all references to these classes so the public utility names consistently
describe the logical alignment behavior.
In `@packages/raystack/styles/typography.css`:
- Line 4: Update the font `@import` declaration in typography.css to use direct
quoted-string notation for the Google Fonts URL instead of the url() wrapper,
preserving the existing URL and import behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 986853d4-b855-4252-befa-8e6315664446
📒 Files selected for processing (48)
packages/raystack/README.mdpackages/raystack/components/accordion/accordion.module.csspackages/raystack/components/announcement-bar/announcement-bar.module.csspackages/raystack/components/avatar/avatar.module.csspackages/raystack/components/button/button.module.csspackages/raystack/components/calendar/calendar.module.csspackages/raystack/components/checkbox/checkbox.module.csspackages/raystack/components/code-block/__tests__/code-block.test.tsxpackages/raystack/components/code-block/code-block.module.csspackages/raystack/components/color-picker/__tests__/color-picker.test.tsxpackages/raystack/components/color-picker/color-picker-area.tsxpackages/raystack/components/color-picker/color-picker.module.csspackages/raystack/components/combobox/combobox.module.csspackages/raystack/components/command/command.module.csspackages/raystack/components/data-view/__tests__/data-view.test.tsxpackages/raystack/components/data-view/components/list.tsxpackages/raystack/components/data-view/data-view.module.csspackages/raystack/components/dialog/dialog.module.csspackages/raystack/components/drawer/drawer-content.tsxpackages/raystack/components/drawer/drawer.module.csspackages/raystack/components/empty-state/empty-state.module.csspackages/raystack/components/field/field.module.csspackages/raystack/components/filter-chip/filter-chip.module.csspackages/raystack/components/headline/headline.module.csspackages/raystack/components/icon-button/icon-button.module.csspackages/raystack/components/image/image.module.csspackages/raystack/components/input/input.module.csspackages/raystack/components/menu/menu.module.csspackages/raystack/components/meter/meter.module.csspackages/raystack/components/popover/popover.module.csspackages/raystack/components/preview-card/preview-card.module.csspackages/raystack/components/progress/progress.module.csspackages/raystack/components/radio/radio.module.csspackages/raystack/components/search/search.module.csspackages/raystack/components/select/__tests__/select.test.tsxpackages/raystack/components/select/select.module.csspackages/raystack/components/sidebar/__tests__/sidebar.test.tsxpackages/raystack/components/sidebar/sidebar.module.csspackages/raystack/components/tabs/tabs.module.csspackages/raystack/components/text-area/text-area.module.csspackages/raystack/components/theme-provider/__tests__/theme.test.tsxpackages/raystack/components/theme-provider/switcher.tsxpackages/raystack/components/tooltip/tooltip.module.csspackages/raystack/components/tour-beta/tour.module.csspackages/raystack/components/tour/tour.module.csspackages/raystack/styles/effects.csspackages/raystack/styles/primitives/accent.csspackages/raystack/styles/typography.css
💤 Files with no reviewable changes (9)
- packages/raystack/components/tour-beta/tour.module.css
- packages/raystack/components/checkbox/checkbox.module.css
- packages/raystack/components/radio/radio.module.css
- packages/raystack/components/field/field.module.css
- packages/raystack/components/progress/progress.module.css
- packages/raystack/components/tabs/tabs.module.css
- packages/raystack/components/meter/meter.module.css
- packages/raystack/components/select/select.module.css
- packages/raystack/components/command/command.module.css
🚧 Files skipped from review as they are similar to previous changes (16)
- packages/raystack/components/accordion/accordion.module.css
- packages/raystack/components/code-block/code-block.module.css
- packages/raystack/components/combobox/combobox.module.css
- packages/raystack/components/text-area/text-area.module.css
- packages/raystack/components/avatar/avatar.module.css
- packages/raystack/components/dialog/dialog.module.css
- packages/raystack/components/preview-card/preview-card.module.css
- packages/raystack/styles/effects.css
- packages/raystack/components/popover/popover.module.css
- packages/raystack/components/data-view/data-view.module.css
- packages/raystack/components/calendar/calendar.module.css
- packages/raystack/components/tour/tour.module.css
- packages/raystack/components/color-picker/color-picker.module.css
- packages/raystack/components/drawer/drawer.module.css
- packages/raystack/components/sidebar/sidebar.module.css
- packages/raystack/components/tooltip/tooltip.module.css
| .action-btn:active { | ||
| transform: scale(var(--rs-scale-pressed)); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Suppress the pressed transform for reduced-motion users.
The scale transform is outside the prefers-reduced-motion: no-preference block, so users requesting reduced motion still receive the press animation. Gate the transform itself or override it under prefers-reduced-motion: reduce.
Proposed fix
-.action-btn:active {
- transform: scale(var(--rs-scale-pressed));
-}
-
`@media` (prefers-reduced-motion: no-preference) {
.action-btn {
transition: var(--rs-transition-interactive);
}
.action-btn:active {
+ transform: scale(var(--rs-scale-pressed));
transition: var(--rs-transition-pressed);
}
}Also applies to: 71-78
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/raystack/components/announcement-bar/announcement-bar.module.css`
around lines 62 - 64, Update the .action-btn:active pressed-state styling so its
scale transform is disabled when prefers-reduced-motion is reduce, while
preserving the existing transform for users with no motion preference. Apply the
same reduced-motion handling to the related styles referenced at lines 71–78.
| const handleRowKeyDown = (event: KeyboardEvent<HTMLDivElement>) => { | ||
| // Enter and Space both activate, matching a native button. | ||
| if (event.key === 'Enter' || event.key === ' ') { | ||
| event.preventDefault(); // Space would otherwise scroll the page. | ||
| handleRowActivate(); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Ignore bubbled keydown events from interactive children.
If the row contains interactive elements (e.g., checkboxes for selection or buttons for row actions), keyboard events from those elements will bubble up to the row. Because native inputs and buttons do not stop propagation for onKeyDown, pressing Enter or Space on a child element will trigger the row's onRowClick and call preventDefault(), which breaks the child element's native keyboard activation.
You can safely ignore these bubbled events by ensuring the keydown target is the row itself.
🛠️ Proposed fix
const handleRowKeyDown = (event: KeyboardEvent<HTMLDivElement>) => {
+ // Ignore events bubbling up from interactive child elements (e.g. selection checkboxes)
+ if (event.target !== event.currentTarget) return;
+
// Enter and Space both activate, matching a native button.
if (event.key === 'Enter' || event.key === ' ') {
event.preventDefault(); // Space would otherwise scroll the page.
handleRowActivate();
}
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleRowKeyDown = (event: KeyboardEvent<HTMLDivElement>) => { | |
| // Enter and Space both activate, matching a native button. | |
| if (event.key === 'Enter' || event.key === ' ') { | |
| event.preventDefault(); // Space would otherwise scroll the page. | |
| handleRowActivate(); | |
| } | |
| }; | |
| const handleRowKeyDown = (event: KeyboardEvent<HTMLDivElement>) => { | |
| // Ignore events bubbling up from interactive child elements (e.g. selection checkboxes) | |
| if (event.target !== event.currentTarget) return; | |
| // Enter and Space both activate, matching a native button. | |
| if (event.key === 'Enter' || event.key === ' ') { | |
| event.preventDefault(); // Space would otherwise scroll the page. | |
| handleRowActivate(); | |
| } | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/raystack/components/data-view/components/list.tsx` around lines 357
- 363, Update handleRowKeyDown to ignore bubbled keydown events unless
event.target is the row element itself, before handling Enter or Space. Preserve
the existing preventDefault and handleRowActivate behavior for direct row
keyboard activation.
| font-family: var(--rs-font-title); | ||
| font-weight: var(--rs-font-weight-regular); | ||
| color: var(--rs-color-foreground-base-primary); | ||
| width: 100%; |
There was a problem hiding this comment.
Issue: Base .headline rule drops width: 100% here (kept only on the truncate variant), with no corresponding entry in docs/V1-migration.md.
Why: The commit itself calls this "BREAKING (mild)" for consumers centering a Headline in a flex/grid row. We've hit this exact regression before on Headline's width contract, breaking a consumer in #799 and fixed a day later in #801. A missing migration-log entry for a breaking change is also this repo's most-repeated review correction across 11 prior PRs.
What: Add an entry to docs/V1-migration.md describing this width-behavior change. Checked not-found.tsx, it's unaffected since it already sets width: auto, but other consumers relying on the old default won't get a heads up without a migration note.
| inset: 0; | ||
| z-index: var(--rs-z-index-portal); | ||
| background-color: var(--rs-color-overlay-black-a5); | ||
| transition: |
There was a problem hiding this comment.
Issue: This .backdrop transition (and the matching .dialogPopup transition/starting-ending-style block further down) gets deleted in this PR's rename to .overlay, with nothing replacing it anywhere in the file.
Why: The commit message only claims to add a prefers-reduced-transparency fallback, it doesn't mention removing the base animation. Sibling Dialog, touched in the very same commit for the same stated purpose, kept its base opacity fade. Right now the Cmd+K palette pops open/closed instantly for everyone, not just reduced-motion users.
What: Restore the opacity (and scale) transition using --rs-duration-*/--rs-ease-* tokens, matching Dialog's pattern.
| role={ariaRole === 'table' ? 'row' : 'listitem'} | ||
| // A clickable row is announced as a button; a plain row keeps its | ||
| // structural role and stays inert for the keyboard. | ||
| role={onRowClick ? 'button' : ariaRole === 'table' ? 'row' : 'listitem'} |
There was a problem hiding this comment.
Issue: For variant="table", a clickable row now gets role="button" instead of role="row", while its cells still render role="cell" and the parent stays role="rowgroup".
Why: ARIA's required-context rule needs cell to have a row ancestor. Once the row reports as a button, most screen readers stop computing column/header association for it. Every sibling clickable-row implementation touched in this same PR (data-table, data-view-beta) kept role="row" for the same case.
What: Keep role="row"/role="cell" intact and add tabIndex/keydown handling alongside it, instead of swapping the role.
| {description && ( | ||
| <FieldPrimitive.Description | ||
| className={styles.description} | ||
| data-hidden={error ? '' : undefined} |
There was a problem hiding this comment.
Issue: FieldPrimitive.Description stays mounted, just hidden via CSS, even when error is also set, for the new crossfade.
Why: Base UI registers every mounted description/error id into aria-describedby regardless of visibility. Verified empirically on both branches: pre-PR aria-describedby resolves to just the error text, post-PR to both the old hint and the error concatenated.
What: Stop rendering the description while an error is showing, or explicitly override aria-describedby to reference only the current message id.
| {icon && ( | ||
| <div className={styles.icon} aria-hidden='true'> | ||
| {icon} | ||
| <div className={styles.transitionShell} data-state={state}> |
There was a problem hiding this comment.
Issue: Two new wrapper divs (.transitionShell, .transitionInner) are added around Callout's root for the exit animation, and .transitionInner sets overflow: hidden unconditionally.
Why: ref now targets the innermost div, not the actual rendered root (verified by rendering <Callout ref={cb}> directly). This is the same "narrowed shared component DOM shape breaks consumers" pattern that's caused two prior incidents in this repo. The unconditional overflow: hidden also clips overflowing content, like a focus ring, even outside the closing state.
What: Scope overflow: hidden to [data-state="closing"] only, and double check whether the ref-target shift is acceptable as is.
| color: var(--rs-color-foreground-base-secondary); | ||
| cursor: pointer; | ||
| text-decoration: none; | ||
| transition: |
There was a problem hiding this comment.
Issue: This transition block on .nav-item gets deleted with no replacement anywhere in the file.
Why: Every other interactive component touched in this PR was migrated onto the shared --rs-transition-interactive token. This one's the miss, so hover/focus/active on the sidebar's most-used control now snaps instead of easing.
What: Add transition: var(--rs-transition-interactive) back, inside the reduced-motion block like its siblings.
| transition: opacity 0.2s ease; | ||
| /* Cap the width so the collapse can animate to 0 (width: auto can't | ||
| transition). 200px clears any real label; the sidebar caps them narrower. */ | ||
| max-width: 200px; |
There was a problem hiding this comment.
Issue: This new max-width cap, and the matching transition it enables, are added so the sidebar collapse can animate, but sidebar-item.tsx/sidebar-more.tsx still conditionally unmount the .nav-text span in the same render the collapse fires.
Why: The element is gone before the transition gets a frame to run, so the label still disappears instantly. .nav-group-label in this same file already solves this correctly by staying mounted unconditionally.
What: Keep the label mounted, matching .nav-group-label's pattern, instead of conditionally unmounting it.
| <IconButton | ||
| aria-label={isDark ? 'Switch to light theme' : 'Switch to dark theme'} | ||
| onClick={onClickHandler} | ||
| style={{ width: size, height: size }} |
There was a problem hiding this comment.
Issue: IconButton's default padding isn't zeroed out here, while the outer element is pixel-locked to size via this inline style.
Why: The icon renders at size - 4px, about 13% smaller than requested by default. The existing "applies custom size" test only checks the SVG's width attribute, not computed layout, so it doesn't catch this.
What: Zero out IconButton's padding when a custom pixel size is supplied, or reconcile the two competing sizing mechanisms.
| {(state.checked || | ||
| state.indeterminate || | ||
| state.transitionStatus === 'ending') && | ||
| (state.indeterminate ? ( |
There was a problem hiding this comment.
Issue: This ternary reads state.indeterminate from Base UI's already-updated state, even while the icon is still fading out via transitionStatus === 'ending'.
Why: When a "select all" checkbox goes indeterminate straight to unchecked, state.indeterminate has already flipped to false by the time the exit transition starts, so it flashes to the checkmark icon for one frame before fading, instead of fading the dash.
What: Track which icon was showing before the transition started, e.g. via a ref, instead of reading current state during the exit.
| @media (prefers-reduced-motion: no-preference) { | ||
| .positioner[data-transition="move"]:has(.popup:not([data-starting-style])) { | ||
| transition: transform 460ms cubic-bezier(0.33, 0.7, 0, 1); | ||
| transition: transform var(--rs-duration-slow) var(--rs-ease-out); |
There was a problem hiding this comment.
Issue: This transition maps to --rs-ease-out, replacing the original hand-tuned cubic-bezier(0.33, 0.7, 0, 1).
Why: That original curve is a near-exact match for the newly-introduced --rs-ease-drawer token instead. The parallel rule in tour-beta was mapped correctly in this same PR, so this looks like a slip during consolidation rather than a deliberate change.
What: Use var(--rs-ease-drawer) instead of var(--rs-ease-out).
| } | ||
|
|
||
| [data-style="modern"] { | ||
| --rs-font-title: var(--rs-font-inter), sans-serif; |
There was a problem hiding this comment.
Issue: These two custom-property redeclarations get removed from [data-style="modern"].
Why: A nested <Theme style="modern"> inside an outer <Theme style="traditional"> now inherits the outer scope's fonts instead of resetting to Inter, silently contradicting the docs' own composition example. This mirrors the gray.css bug this team already fixed once before (#768).
What: Keep the explicit --rs-font-title/--rs-font-body re-declaration here.
| return <tr {...props} />; | ||
| const row = cva(styles['row'], { | ||
| variants: { | ||
| interactive: { |
There was a problem hiding this comment.
Issue: This new interactive variant on TableRow isn't reflected in apps/www/.../table/props.ts.
Why: It's a real, user-facing behavioral prop, toggles hover/active row styling via CSS, and this repo's convention is to update docs props.ts in the same PR a component gains a new prop.
What: Add interactive to TableRowProps in the docs site.
| aria-valuetext={valueText} | ||
| aria-valuemin={0} | ||
| aria-valuemax={100} | ||
| aria-valuenow={Math.round((chroma / CHROMA_MAX) * 100)} |
There was a problem hiding this comment.
Issue: role="slider" here reports only this one axis (chroma) via aria-valuenow/min/max, but the keyboard handler drives a second, independent axis (lightness) that never shows up in these values.
Why: aria-valuetext does update correctly on every keypress, so this isn't a functional break, but it's non-conformant ARIA for a genuinely 2D control.
What: Consider two separate slider elements, one per axis, matching established 2D color-area patterns like React Aria's ColorArea.
| <span | ||
| className={styles.datePickerError} | ||
| data-visible={error ? '' : undefined} | ||
| role='alert' |
There was a problem hiding this comment.
Issue: This new error span has no aria-describedby link connecting it to the input.
Why: role="alert" only announces the message at the moment it appears. A user tabbing into an already-erroring field hears aria-invalid with no stated reason.
What: Give the error span a stable id and wire it into the input's aria-describedby.
| } | ||
|
|
||
| .action-btn:focus-visible { | ||
| outline: 1px solid currentColor; |
There was a problem hiding this comment.
Issue: This focus ring hardcodes outline/currentColor instead of the new shared --rs-focus-ring token used by roughly 19 other components in this PR.
Why: Turns out deliberate, currentColor reads better against the bar's colored variants per the commit message, but that reasoning lives only there, not as a code comment, so a future token-adoption pass is likely to "fix" it by mistake.
What: Add a one-line comment here explaining the divergence.
| const sawLoadingRef = useRef(false); | ||
| const [fadeIn, setFadeIn] = useState(false); | ||
| const handleLoadingStatusChange = (status: ImageLoadingStatus) => { | ||
| if (status === 'loading') { | ||
| sawLoadingRef.current = true; | ||
| setFadeIn(false); | ||
| } else if (status === 'loaded') { | ||
| // A cached image reports 'loaded' with no 'loading' phase (Base UI never | ||
| // emits it), so it stays instant; only network loads fade in. | ||
| setFadeIn(sawLoadingRef.current); | ||
| sawLoadingRef.current = false; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Base UI Avatar has data-starting-style css variable which can be used to fade the image. Let's use that
| .action-btn:hover { | ||
| opacity: 0.8; | ||
| } | ||
|
|
||
| /* Press reads as scale, not a second dim — 0.64 text mid-press is barely | ||
| legible on the gradient variant, and hover's 0.8 still applies. */ | ||
| .action-btn:active { | ||
| transform: scale(var(--rs-scale-pressed)); | ||
| } | ||
|
|
||
| .action-btn:focus-visible { | ||
| outline: 1px solid currentColor; | ||
| outline-offset: 2px; | ||
| } |
There was a problem hiding this comment.
Right now the action-btn is a html button. Ideally it should be using Apsara Button component, in that way we don't have to add focus styles in other components.
| --rs-ease-out: cubic-bezier( | ||
| 0.22, | ||
| 1, | ||
| 0.36, | ||
| 1 | ||
| ); /* entrances, exits, value changes */ | ||
| --rs-ease-in-out: cubic-bezier( | ||
| 0.77, | ||
| 0, | ||
| 0.175, | ||
| 1 |
There was a problem hiding this comment.
lines need to be prettified/formatted
| .breadcrumb-link:focus-visible { | ||
| outline: none; | ||
| background-color: var(--rs-color-background-base-primary-hover); | ||
| color: var(--rs-color-foreground-base-secondary); | ||
| border-radius: var(--rs-radius-1); | ||
| } | ||
|
|
| .breadcrumb-dropdown-trigger:focus-visible { | ||
| background-color: var(--rs-color-background-base-primary-hover); | ||
| color: var(--rs-color-foreground-base-primary); | ||
| border-radius: var(--rs-radius-1); | ||
| } | ||
|
|
| /* Trigger sits flush in a clipped container: keep the ring inside. */ | ||
| .accordion-trigger:focus-visible { | ||
| outline: var(--rs-focus-ring); | ||
| outline-offset: var(--rs-focus-ring-offset-inset); |
| default; a colored control (danger, success, …) overrides | ||
| `outline-color` in its own :focus-visible rule to match. */ | ||
| --rs-focus-ring: 1px solid var(--rs-color-border-accent-emphasis); | ||
| --rs-focus-ring-shadow: 0 0 0 1.5px var(--rs-color-border-accent-primary); |
There was a problem hiding this comment.
| --rs-focus-ring-shadow: 0 0 0 1.5px var(--rs-color-border-accent-primary); | |
| --rs-focus-ring-shadow: 0 0 0 1px var(--rs-color-border-accent-emphasis); |
^ This is used mainly for borderless input variants. We should match this style to the ring css for consistency.



Summary
A design-system-wide audit of the component library, consolidating the animation work and the broader interaction/craft pass into a single branch.
What's covered
:focus-visiblering across the library.styles/effects.css; Inter body tracking and the h1–h4 type ramp retuned.Related issues
Closes #681 — consistent, tokenized
:focus-visiblering applied across the library.Closes #621 — Image error/loading handling: one-shot fallback guard plus load-state fade-in.
Closes #625 — Link's remaining items (focus-visible, reduced-motion) are now done.
Partially addresses:
TableRownow uses CVA and gains row-state hooks. Cell truncation accessibility is still open.announementBartypo are still open.Test plan
pnpm buildpasses.@base-uimodule-resolution failure affects a handful of unrelated test files onmainas well.